Skip to content

fix(ui-web): let a direct sub-agent chat be stopped from its own surface - #1140

Merged
0xKT merged 1 commit into
EverMind-AI:mainfrom
kaguranananana:fix/subagent_instance_turn_cancel
Nov 2, 2026
Merged

0xKT merged 1 commit into
EverMind-AI:mainfrom
kaguranananana:fix/subagent_instance_turn_cancel

Conversation

@kaguranananana

Copy link
Copy Markdown
Contributor

Summary

A turn sent to a sub-agent instance could not be stopped. The instance surface exposed no cancel, and the global stop cannot reach one: it sends turn.cancel with no target, and the server resolves the lane from that target, so the lookup misses every direct turn - they are registered under direct_lane(...), never under the bare session key (#459). The backend has accepted a target all along; the page just never wired it.

This adds instanceCancel to the subagents source, calling turn.cancel with the (agent, handle) pair that names the lane, and draws a stop button on the instance composer while that instance is answering - beside the send button, which keeps its own job of queueing while a turn runs. The busy state clears on the cancelled turn's own error frame, which arrives tagged with the target and settles like a completed turn's, so a refused cancel needs no local undo. A stop whose call fails surfaces its reason in the same line a refused send does. Queued messages stay queued and run after the stop, matching the page composer's own stop semantics.

The stop button reuses the shared .go send-button class with data-state="stop" as the marker, the way the page composer's go button names its own state - no new class in the page's namespace.

Type

  • Fix
  • Feature
  • Docs
  • CI / tooling
  • Refactor
  • Other

Verification

  • Relevant tests pass locally
  • Relevant lint / type checks pass locally
  • User-facing docs or screenshots are updated when needed

Commands run (node 24 from /opt/homebrew/opt/node@24, in ui-web/ unless noted):

  • npx vitest run src/features/subagents/ - 136 passed (5 files)
  • npx vitest run - 260 files, 4432 tests passed (4428 before; the 4 new regression tests are 3 in SubagentsPage.test.tsx, 1 in source.test.ts)
  • Falsification: source fix temporarily reverted, npx vitest run src/features/subagents/SubagentsPage.test.tsx src/features/subagents/source.test.ts -t "stop" - 3 failed; fix restored - all pass
  • npx tsc --noEmit -p tsconfig.json - exit 0
  • npx eslint src/features/subagents/ - 0 errors (2 pre-existing react-hooks warnings, verified present on the unmodified file)
  • node scripts/check-class-namespace.mjs - OK
  • uv run python scripts/check_source_language.py origin/main - exit 0 (this run compares the working tree against origin/main, which is what caught and rejected an earlier CJK test fixture; the fixture text is English now)
  • uv run pytest tests/test_rpc_turn_cancel.py -q (repo root) - 34 passed, including test_a_direct_turn_is_cancelled_through_its_target, the backend contract this wires up

No docs update needed: the stop is the same control the page composer already teaches.

Risk

  • Security impact considered - none: the call goes to the same turn.cancel the page composer already sends, now addressed; no new surface
  • Backward compatibility considered - the source method is optional like its siblings, so a page against a server without the capability draws no stop button, covered by a test
  • Rollback path is clear - revert of this single commit restores the previous behavior

Related Issues

Fixes #459

A turn sent to a sub-agent instance could not be stopped. The instance
surface exposed no cancel, and the global stop cannot reach one: it
sends turn.cancel with no target, and the server resolves the lane from
that target, so the lookup misses every direct turn -- they are
registered under direct_lane(...), never under the bare session key.
The backend has accepted a target all along; the page just never wired
it.

Add instanceCancel to the subagents source, calling turn.cancel with
the (agent, handle) pair that names the lane, and draw a stop button on
the instance composer while that instance is answering -- beside the
send button, which keeps its own job of queueing while a turn runs.
The busy state clears on the cancelled turn's own error frame, which
arrives tagged with the target and settles like a completed turn's, so
a refused cancel needs no local undo. A stop whose call fails surfaces
its reason in the same line a refused send does.

Co-authored-by: GLM (GLM-5.3-Flash) <noreply@z.ai>
@LivXue

LivXue commented Oct 28, 2026

Copy link
Copy Markdown
Member

Windows compatibility review — PR #1140 @ 03c5fec0c1b1e330a04f5198105610c4d6fa0a4d

Scope: full base 06509f8e → this head (6 files, +144): subagents/source.ts gains instanceCancel (the same turn.cancel the page composer uses, with the (agent, handle) target that names a direct turn's lane), store.ts a cancelInstanceTurn/canCancel, the instance composer a stop button, and tests.

Result: no Windows-specific issue. The change is DOM/state wiring plus an RPC parameter shape; no path, filesystem, subprocess, signal, encoding, or platform primitive.

Verification (native Windows 11, isolated worktree D:\tmp\rv-1140 at this head; node_modules borrowed read-only from the main checkout):

vitest run src/features/subagents/SubagentsPage.test.tsx src/features/subagents/source.test.ts
→ 2 files, 101 passed

Limitations: no real-browser walkthrough and no live gateway; the cancel path is covered through the mocked source.

@LivXue
LivXue requested review from LivXue and gloryfromca October 29, 2026 12:14

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

Reviewed the full github/main...HEAD diff and the surrounding direct-chat lifecycle. The new source method sends turn.cancel with the same (agent, handle) target used to resolve direct-turn lanes, and the backend tags the cancellation event back to that target, so the existing direct event path clears the busy state and continues any queued work. The optional source capability preserves compatibility with hosts that cannot cancel.

Coverage included AGENTS.md and the Web UI context/architecture rules, the changed code and callers, relevant backend cancellation and event-routing code, nearby history, backward compatibility, test integrity, and a dry merge against the current target branch. No tests were weakened.

Verification:

  • npm test -- --run src/features/subagents/source.test.ts src/features/subagents/SubagentsPage.test.tsx: 2 files, 101 tests passed
  • npm run type-check: passed
  • git diff --check github/main...HEAD: passed
  • git merge-tree --write-tree HEAD github/main: clean

@LivXue

LivXue commented Oct 29, 2026

Copy link
Copy Markdown
Member

No blockers in the reviewed scope.

Scope: I reviewed only the raven/agent/** portion of this PR -- 0 of 6 changed files. The remaining 6 files (all under ui-web/src/features/subagents/) were read as context for this verdict but are not covered by it, and no path in this PR accepts an inline note. What follows is the scope declaration plus observations disclosed here rather than anchored on a file.

What I examined: the base-to-head diff, the direct-lane cancel path this depends on (TurnCancelParams.target, direct_lane resolution, the error frame tagged back to the target, and the tests that pin them), the subagents source and store, the instance composer, and the merge state at this head.

Confirmed findings: none in the reviewed scope.

Disclosed observations (outside the reviewed scope, none blocking):

  • The surface that now has a stop does not have a fresh entry in the current desk-era page. The button renders only inside InstanceComposer when chat.busy && canCancel() (ui-web/src/features/subagents/SubagentsPage.tsx:605), and that composer is hosted by a pane opened only through openInstance -> openDeskAgent. On main, .desk-ready .ws > .ws-top and .ws > .ws-body hide the workspace pane (ui-web/src/styles/page.css:5411-5412), #wsBtn is visibility: hidden (page.css:5424), and the desk's tabs are only deliverables/tasks/diff (ui-web/src/features/desk/store.ts:141); ws.pick('agents') is downgraded by openDeskTab to opening the palette without changing the tab (desk/store.ts:528-535). Direct chats still reopen through session replay (ui-web/src/state/session/resume.ts:82-86) and remain fully usable from the TUI, which has sent turn.cancel with target since this contract existed (ui-tui/src/app/chatStream.ts:881-886). So this PR is the correct web half of that parity, and its user-visible effect is limited to panes already open until an entry returns. Not introduced by this PR, and not something I am filing against it; it is disclosed so the merge is not read as closing A direct sub-agent chat cannot be stopped: no cancel on the instance surface, and the global stop omits the target #459 for users.

  • A pane opened after a turn has started shows no stop: busy is seeded only by a local send (store.ts:568) or a message.start frame (store.ts:614), never from the row's turnStartedAtMs, which the server already publishes while a direct turn is answering (raven/rpc/methods/instances.py:333-357, raven/agent/subagent/manager.py:1260). Code-reading; not reproduced in a browser.

  • cancelInstanceTurn discards the {cancelled, busy} reply (store.ts:688-694). The page composer reads it because the reply lands before the coalesced frames (ui-web/src/state/session/runtime.ts:430-465); a direct turn self-heals from its own end frame, so this is a recorded design difference, not a defect.

Verification (run at 03c5fec in a detached worktree of this head, node 26, Linux):

  • npx vitest run src/features/subagents/source.test.ts src/features/subagents/SubagentsPage.test.tsx -> 2 files, 101 tests passed.
  • npx tsc --noEmit -p tsconfig.json -> exit 0.
  • npx eslint src/features/subagents/ -> 0 errors, 2 pre-existing react-hooks warnings at SubagentsPage.tsx:370 and :667, both outside the added hunks.
  • The backend contract this relies on is pinned by tests/test_rpc_turn_cancel.py, including the targeted cancel and the one-instance isolation cases.
  • The observations above are code-reading plus a live RPC read of subagents.instances (resumable: true for the Raven-General rows); no browser reproduction.

Checked and deliberately not reported:

  • The backend accepts the target and resolves direct_lane(session_key, agent, handle); a successful cancel emits an error tagged with that target, and the client routes tagged frames to directEvent and returns, so the host transcript never sees it.
  • gui.stop exists in both catalogue columns.
  • No CSS is added; the new button reuses .go with data-state="stop", which has no rule of its own, matching the page composer's marker.
  • The optional method keeps old servers drawing no stop button, and a test covers it.
  • The target cannot cross sessions: it resolves a lane inside the named session, the same scope turn.send already had.
  • Main is 7 commits ahead of the base; fix(ui-web): keep a late delta out of the next turn's step #1107 touches state/session/stages.ts but not the target routing this PR depends on.

@0xKT
0xKT merged commit 598f2f4 into EverMind-AI:main Nov 2, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A direct sub-agent chat cannot be stopped: no cancel on the instance surface, and the global stop omits the target

4 participants